fix(server-nestjs): make Keycloak root group creation idempotent - #2623
fix(server-nestjs): make Keycloak root group creation idempotent#2623shikanime wants to merge 1 commit into
Conversation
dd18554 to
0a0efb2
Compare
Updated reviewThe fast-path read at line 187 is intentional — when the root group exists but the full path does not (e.g. I initially flagged the read as redundant, and it IS technically redundant for single-part paths ( The PR is correct as-is. |
shikanime
left a comment
shikanime
left a comment
There was a problem hiding this comment.
Verdict : Commentaire non bloquant — logique correcte mais redondante avec main (PR draft).
- keycloak-client.service.ts:128-137 — [🟡 Nit] La gestion du 409 dans
createGroup(re-fetch viagetRootGroupByName) est correcte et alignée sur le comportement existant des sous-groupes. En revanche elle dupliquegetErrorResponseStatus(err) !== 409qui se trouve DÉJÀ surmain(commitec9c18a914, lignes 214-215). Rebasez surmain: le diff ne devrait contenir que le test unitaire (la logique y est déjà). - keycloak-client.service.spec.ts:373+ — [✨ Éloge] Le test de course 409 (deux lectures vides, create 409, re-fetch du groupe racine concurrent) couvre précisément #2618.
Sortir du draft uniquement après rebase sur main (la logique 409 est déjà mergée).
… race) Refs #2618 Co-authored-by: Automata <automata@shikanime.studio> Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: I5944dfc5a50f280ef836ce9e9a136f366a6a6964
0a0efb2 to
ab38b80
Compare
|
0 New Issues
1 Fixed Issue
0 Accepted Issues1 participant
Issues liées
#2618 (fermer délibérément après la fusion)
Quel est le comportement actuel ?
La création du groupe racine Keycloak n'est pas idempotente face à une course concurrente.
getOrCreateGroupByPathlit d'abord le groupe racine, puis appellecreateGroupsi celui-ci est absent. Or deux réconciliations simultanées (la synchronisation cron et unproject.upsert) peuvent toutes deux passer la lecture avant qu'aucune n'ait créé le groupe : la seconde échoue alors avec une erreur HTTP 409, car le groupe existe désormais.createGroupne tolérait pas ce 409 et remontait l'erreur au lieu de récupérer le groupe déjà créé.Comportement attendu
Un 409 sur la création du groupe racine doit être traité comme « le groupe existe déjà » : le groupe existant est re-consulté via
getRootGroupByNamepuis renvoyé, exactement de la même manière que le chemin des sous-groupes (getOrCreateSubGroupByName) le fait déjà. Aucun autre comportement ne change.Changements
client.groups.createdanscreateGroupavec une tolérance au 409 : en cas de 409, re-consulter le groupe racine existant et le renvoyer.